feat(schematics): rewrite Vertex AI imports to AI Logic on ng update - #3725

Nice work on the AST approach — the two-pass design holds up well under poking. The namespace and identifier passes can't double-edit the same token (isMemberOrDeclaredName catches the child visit in both value and type position), applyEdits sorting back-to-front is right, and the spec coverage is genuinely thorough: shorthand expansion, accessor look-alikes, destructuring keys vs. binding initializers, idempotency. *.jasmine.ts is picked up by tools/jasmine.ts, so these run in CI. The symbol map matches the real src/ai/public_api.ts exports for every entry except one — which is the blocker below.

1. getVertexAI → getAI is not a rename, and the rewrite silently changes which backend the app calls

getVertexAI and getAI both existed in the old module. git show ac3dd7c^:src/vertexai/firebase.ts exports all four of getAI, getVertexAI, getGenerativeModel, getImagenModel. Two coexisting functions aren't a rename pair.

getAI() with no options defaults to the Google AI (Gemini Developer API) backend; the Vertex equivalent is getAI(app, { backend: new VertexAIBackend() }). So a user on Vertex who runs ng update ends up with code that compiles and then talks to a different API, with different enablement and billing. Nothing in the diff would look wrong on review, which defeats the "ng update always shows its changes as a diff" safety net cited under Known limitation.

Two ways out:

• rewrite getVertexAI(...) to getAI(app, { backend: new VertexAIBackend() }), adding VertexAIBackend to the rewritten import, or
• leave getVertexAI calls alone and context.logger.warn with a pointer to the upgrade guide.

Worth confirming the SDK's default backend against @firebase/ai before picking. The symbol table in docs/version-21-upgrade.md and the note added to docs/ai.md need the same correction — as written, both teach the rename as safe.

2. export { VertexAI } from '@angular/fire/vertexai' renames the user's public export

src/schematics/update/v21/vertexai-to-ai.ts:181 — collectSpecifierEdit rewrites importedNameNode, which is element.name when there's no propertyName. For a re-export with a from clause that produces export { AI } from '@angular/fire/ai', so the file's external export name changes from VertexAI to AI and downstream consumers break.

identifierUsageEdit already handles this correctly for the bare local re-export — expanding to export { AI as VertexAI }, with a good comment explaining why. The from-clause path should do the same. (export { VertexAI as Foo } from '...' is already correct, since propertyName is set.)

3. The migration walks node_modules when a project has no sourceRoot

rewriteVertexAIToAI derives roots from sourceRoot || root. A root project with root: "" and no sourceRoot — which older CLI versions generate — gives posix.join('/', '') → /, and every path in the tree passes the prefix test. The content.includes(specifier) prefilter keeps most files from being parsed, but any dependency that re-exports @angular/fire/vertexai gets rewritten in place inside node_modules.

A filePath.includes('/node_modules/') guard is the usual fix and costs nothing.

4. Step ordering interacts badly with the typescript external

Making typescript an esbuild external is the right call for package size, but it isn't declared anywhere in src/package.json — unlike firebase-tools, which is an optional peer. Under a strict or isolated node_modules layout the top-level import * as ts fails at module load, which takes down the whole migration.

Because src/schematics/update/v21/index.ts:14 runs the rewrite before alignFirebaseVersion, that failure also costs users the firebase-12 alignment — the part they genuinely can't do without. Two small changes: declare typescript as an optional peer dependency, and run alignFirebaseVersion first so a rewrite failure can only cost you the rewrite.

5. Not covered: direct firebase/vertexai imports

Since this migration also moves users to Firebase JS SDK 12, where that entry point is gone, direct SDK imports break too. The guide flags it as manual, but the same AST pass would handle it with one more entry in OLD_MODULE_SPECIFIERS — worth considering, subject to the same getVertexAI caveat above.

Merge order

This is stacked on unmerged predecessor work, and it edits the same README feature-table region as #3724 (that PR re-flows the row boundaries around the last two cells; this one rewrites the Vertex AI cell's content). Whichever lands second will need a manual rebase.

Marking this as changes-requested to hold the merge, per my earlier review.

The blocker is item 1: getVertexAI and getAI coexisted in the old module (git show ac3dd7c^:src/vertexai/firebase.ts exports both), so rewriting one to the other isn't a rename — it moves a Vertex user onto the Google AI backend silently, and the resulting diff looks correct on inspection. That needs resolving in the migration and in both docs pages before this lands.

Items 2-4 (the export { X } from public-export rename, the node_modules walk when sourceRoot is absent, and the typescript external / step-ordering interaction) are smaller but concrete. Item 5 is optional.

Happy to re-review once the backend question is settled.

Approving. I pulled the branch and verified rather than taking the summary on faith: npm run build succeeds, ng lint is clean apart from the pre-existing @ts-ignore in deploy/actions.ts, and the node suite is 117 specs / 0 failures, matching your numbers (60 in vertexai-to-ai.jasmine.ts plus 4 in index.jasmine.ts).

On item 3, you are right and my diagnosis was wrong. The original .filter((base: string) => !!base) drops '' before it reaches posix.join, so a root: "" project never produced a / root, and your second point holds too: startsWith(root + '/') builds '//' for a / root and matches nothing. There was no node_modules walk. The real bug was the inverse of what I described, a root project silently skipped and never migrated, and you found and fixed it. Your !filePath.split('/').includes('node_modules') is also better than the .includes('/node_modules/') I suggested, and it is genuinely load-bearing now that / is a legal root.

Item 1 checks out end to end. I confirmed options?.backend ?? new GoogleAIBackend() in @firebase/ai myself, so the original rewrite really did move callers onto the wrong backend. One detail your summary does not mention that I checked, because it decides whether the rewrite is truly behavior preserving: legacy @firebase/vertexai used DEFAULT_LOCATION = 'us-central1' and VertexAIBackend's constructor defaults to the same value, so getVertexAI(app) becoming getAI(app, { backend: new VertexAIBackend() }) keeps the region identical. The zero-argument case is right for the same kind of reason, since getAI's signature is getAI(app = getApp(), options) and an explicit undefined triggers the default parameter. Items 2, 4, and 5 all match what you describe in the code.

The "nothing changes behavior silently" rule is the right one to have landed on, and the per-site warnings plus the moved import path make the failures loud in the place the user needs to look.

One observation, not a request: this grew from roughly 460 lines to 2,394, and a good part of that is hardening beyond the blocker. It is the right hardening and I am not asking you to unpick it now. But a change that arrives at this size is hard to review as a unit, and next time the safety analyses and the optional item 5 work would probably be easier on a reviewer as a follow-up on top of the core fix. Worth keeping in mind rather than acting on here.